Conversation
There was a problem hiding this comment.
Pull request overview
Updates lock-file frontmatter hash extraction to support the newer JSON metadata header format while maintaining backward compatibility with the legacy # frontmatter-hash: line.
Changes:
- Update
extractHashFromLockFileto read# gh-aw-metadata: {...}viaworkflow.ExtractMetadataFromLockFile, with fallback to legacy parsing. - Add a unit test covering hash match/mismatch detection using the JSON metadata format.
- Clarify docs wording for when the GitHub Projects token is required.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| pkg/cli/run_push.go | Uses workflow lock metadata parsing to extract frontmatter_hash from JSON metadata, falling back to legacy extraction. |
| pkg/cli/run_push_test.go | Adds coverage ensuring JSON metadata hashes are correctly interpreted for match/mismatch. |
| docs/src/content/docs/reference/auth.mdx | Clarifies “When Required” conditions for the Projects token. |
Comments suppressed due to low confidence (3)
pkg/cli/run_push.go:650
ExtractMetadataFromLockFilecan return a non-nil error for malformed# gh-aw-metadata: {...}JSON, but this code silently ignores that and falls back to legacy parsing. That can lead to misleading observability (e.g., logging "No frontmatter-hash found" even though metadata was present but invalid). Consider logging the metadata parse error (or returning it up tocheckFrontmatterHashMismatch) before falling back so failures are visible.
// First, try to extract from JSON metadata format using the proper workflow package function
if metadata, _, err := workflow.ExtractMetadataFromLockFile(content); err == nil && metadata != nil {
if metadata.FrontmatterHash != "" {
return metadata.FrontmatterHash
}
pkg/cli/run_push.go:17
- The import block is split into two internal-package groups (blank line after
pkg/stringutil), which is inconsistent with otherpkg/clifiles (e.g.,pkg/cli/access_log.go:10-14) and may get rewritten by goimports. Consider grouping allgithub.com/github/gh-aw/pkg/...imports together in a single block.
"github.com/github/gh-aw/pkg/stringutil"
"github.com/github/gh-aw/pkg/console"
"github.com/github/gh-aw/pkg/fileutil"
"github.com/github/gh-aw/pkg/logger"
pkg/cli/run_push_test.go:423
- This test fixture/comment says the JSON metadata line is "as written by current compiler", but the compiler JSON uses
omitemptyforstop_time(seepkg/workflow/lock_schema.go:24-28), so an empty stop_time field typically won't be emitted. Consider either omittingstop_timefrom the test lock content or giving it a non-empty value to better match real output.
// Create a lock file with JSON metadata format (as written by current compiler)
// Uses snake_case field names matching the Go struct JSON tags
lockFilePath := filepath.Join(tmpDir, "test-workflow.lock.yml")
lockContent := fmt.Sprintf(`# This workflow was auto-generated by gh-aw
#
# gh-aw-metadata: {"schema_version":"v1","frontmatter_hash":"%s","stop_time":""}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Hey @dsyme 👋 — thanks for jumping in to fix the frontmatter hash extraction for the new JSON metadata format! The technical changes look solid: you've added tests for both matching and non-matching hashes, updated the extraction logic to handle both formats, and maintained backward compatibility with the legacy format. However, this repository has a unique contribution process that this PR doesn't follow:
Why this matters: This project practices "dogfooding" — using agentic workflows to build agentic workflows. By creating PRs directly, we bypass the very system we're building, which defeats the purpose and prevents us from discovering issues in the agentic workflow process itself. Next steps: A maintainer will need to decide whether to accept this PR as an exception or ask you to close it and follow the standard agentic contribution process. Thanks again for the contribution — the fix itself is well-implemented! 🙌
|
Summary
extractHashFromLockFileto first attempt parsing the new JSON metadata format (# gh-aw-metadata: {...}) before falling back to the legacy# frontmatter-hash: <hash>format